Added keybinding for renaming group with F2 by Rydzard · Pull Request #12159 · JabRef/jabref

Rydzard · 2024-11-05T12:47:57Z

Describe the changes you have made here: what, why, ...
Link the issue that will be closed, e.g., "Closes #333". If your PR closes a koppor issue, link it using its URL, e.g., "Closes JabRef#47".

Mandatory checks

I own the copyright of the code submitted and I licence it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Rydzard · 2024-11-05T12:49:58Z

Hello. I added the Rename Group to the context menu with keybinding (for now I set it to the letter R, because I couldn't try F2 locally, I will change it to F2 in the finale). I just have one problem, that when I change the name of the group, the groups become smaller (I will also send a video example). Can you advise me why it does this and how I could fix it?

Video : https://github.com/user-attachments/assets/396d439e-ca00-451c-991b-43f12666bc72

github-actions

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

Siedlerchr · 2024-11-06T23:25:02Z

Code wise looks okay to me so far, but I haven't an idea why the group size changes. Does this happen with the normal dialog as well?
Can you pleaes fix the failing checkstyle test?

github-actions

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

koppor · 2024-11-11T23:13:29Z

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

+    /**
+     * Opens "Rename Group Dialog" and change name of group
+     */
+


Why the empt yline?

Removed empty line

koppor · 2024-11-11T23:13:40Z

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

+    public void renameGroup(GroupNodeViewModel oldGroup) {
+        currentDatabase.ifPresent(database -> {
+            AbstractGroup oldGroupDef = oldGroup.getGroupNode().getGroup();
+            String oldGroupName = oldGroupDef.getName(); // Zachytenie starého názvu pred otvorením dialógu


Please translate the comment (or remove it)

koppor · 2024-11-11T23:14:22Z

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

+                    return;
+                }
+
+                // check if is not include ","


Remove comment - the next line says the same.

The WHY is important. Why is this check done.

Hello. I removed this comment. I check this because I inspired by Edit group where this check too. I can remove this if-statement if you want.

koppor · 2024-11-11T23:14:31Z

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

+                    return;
+                }
+
+                // chceck if old group name dont equels with new group name


Remove comment - next line says it

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

koppor · 2024-11-11T23:16:33Z

src/main/java/org/jabref/gui/groups/RenameGroupView.java

+        nameField = new TextField();
+        nameField.setPromptText("Enter new group name");
+
+        // add Input


koppor · 2024-11-11T23:16:47Z

src/main/java/org/jabref/gui/groups/RenameGroupView.java

+        getDialogPane().getButtonTypes().setAll(ButtonType.OK, ButtonType.CANCEL);
+
+        nameField = new TextField();
+        nameField.setPromptText("Enter new group name");


Use Localiaztion.lang - see https://devdocs.jabref.org/code-howtos/localization.html

koppor · 2024-11-11T23:16:53Z

src/main/java/org/jabref/gui/groups/RenameGroupView.java

+        nameField.setPromptText("Enter new group name");
+
+        // add Input
+        VBox vbox = new VBox(new Label("New group name:"), nameField);


Use Localiaztion.lang - see https://devdocs.jabref.org/code-howtos/localization.html

No ":"

koppor · 2024-11-11T23:16:59Z

src/main/java/org/jabref/gui/groups/RenameGroupView.java

+        VBox vbox = new VBox(new Label("New group name:"), nameField);
+        getDialogPane().setContent(vbox);
+
+        // If press OK change name else return null


koppor · 2024-11-11T23:17:03Z

src/main/java/org/jabref/gui/groups/RenameGroupView.java

+            if (buttonType == ButtonType.OK) {
+                return resultConverter(ButtonType.OK).orElse(null);
+            } else {
+                // Ak sa zvolí Cancel alebo sa dialóg zavrie cez X


github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

…renameGroup

Rydzard · 2024-11-18T21:02:44Z

Hello @koppor @Siedlerchr . I have a small problem with Unit Test findMissingLocalizationKeys(). I added strings to JabRef_en.properties, but this test on my local computer still shows error that I dont have string in this file. Can someone tell, what I have done wrong please?

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

Siedlerchr · 2024-11-19T16:23:55Z

That looks correct at first sight . Maybe some caching. Can you push the modifications? Then we can see if it works on CI

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

koppor · 2024-11-25T22:21:57Z

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

+                if (newGroupName.contains(",")) {
+                    return;
+                }


Doesn't JabRef have a check for new group names?

What about the dialog to add new groups?

If the check is also in there, maybe, the method can be extracted?

koppor · 2024-11-25T22:22:08Z

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

+                if (oldGroupName.equals(newGroupName)) {
+                    return;
+                }


That should go first.

koppor · 2024-11-25T22:22:22Z

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

+            );
+
+            newGroup.ifPresent(group -> {
+


Pleaase - not that many empty lines. Especially not after {.

koppor · 2024-11-25T22:24:07Z

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

+                int groupsWithSameName = 0;
+                Optional<GroupTreeNode> databaseRootGroup = currentDatabase.get().getMetaData().getGroups();
+                if (databaseRootGroup.isPresent()) {
+                    groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(newGroupName)).size();


There needs to be comment why this is necessary. At first guess, I would have thought that there must not be any other group with the same name. Why is one other group with the same name OK?

Still valid comment. I don't see any comment in the Java code. Thus, I cannot judge - and future code readers should have a clue, too!

github-actions

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

koppor

F2 does not work here. (windows 10)
Dialog does not show current group name
Dialog does not focus text field for new group name

It is not clear which issue is addressed.

Best is to trigger "Edit group dialog" on press of F2.

Alternatively, an in-line editing at the groups tree can be done. Similar to "file rename" in VS.Code.

This PR does not do any of these two, therefore, I close this PR.

Feel free to re-open if you intend to work on these issues.

koppor · 2025-02-16T14:56:21Z

src/main/java/org/jabref/gui/groups/GroupDialogView.java

                           @Nullable GroupTreeNode parentNode,
                           @Nullable AbstractGroup editedGroup,
                           GroupDialogHeader groupDialogHeader) {
+


Why adding a new line?

koppor · 2025-02-16T14:56:30Z

src/main/java/org/jabref/gui/groups/GroupDialogView.java

    private final EnumMap<GroupHierarchyType, String> hierarchyToolTip = new EnumMap<>(GroupHierarchyType.class);

    private final ControlsFxVisualizer validationVisualizer = new ControlsFxVisualizer();
-


This removal of the empty line is OK.

koppor · 2025-02-16T14:57:09Z

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

+                int groupsWithSameName = 0;
+                Optional<GroupTreeNode> databaseRootGroup = currentDatabase.get().getMetaData().getGroups();
+                if (databaseRootGroup.isPresent()) {
+                    groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(newGroupName)).size();


Still valid comment. I don't see any comment in the Java code. Thus, I cannot judge - and future code readers should have a clue, too!

Added keybinding for renaming group with F2

42616e2

Rydzard mentioned this pull request Nov 5, 2024

Add "Rename group" to group context menu and add keybinding for F2 #11896

Closed

github-actions bot reviewed Nov 5, 2024

View reviewed changes

fix group size changes

ea811f1

github-actions bot reviewed Nov 11, 2024

View reviewed changes

fix checkstyleTest

8144433

github-actions bot reviewed Nov 11, 2024

View reviewed changes

Discard changes to src/main/java/org/jabref/gui/DialogService.java

20c387a

koppor requested changes Nov 11, 2024

View reviewed changes

koppor added the status: changes-required Pull requests that are not yet complete label Nov 11, 2024

github-actions bot reviewed Nov 11, 2024

View reviewed changes

Richard Dulák added 2 commits November 18, 2024 21:35

remove comments etc.

67a5d5f

Merge branch 'renameGroup' of https://github.com/Rydzard/jabref into …

5dcc5a2

…renameGroup

remove comments etc.

487754a

github-actions bot reviewed Nov 18, 2024

View reviewed changes

Richard Dulák and others added 2 commits November 22, 2024 21:51

try CI

a3e8599

Merge branch 'main' into renameGroup

bfa8228

github-actions bot reviewed Nov 22, 2024

View reviewed changes

DeleteTest

645693b

github-actions bot reviewed Nov 22, 2024

View reviewed changes

return FetcherTest

75f84bc

github-actions bot reviewed Nov 22, 2024

View reviewed changes

Update JabRef_en.properties

068253c

Rydzard requested a review from koppor November 22, 2024 23:17

koppor requested changes Nov 25, 2024

View reviewed changes

koppor marked this pull request as draft December 2, 2024 20:03

Rydzard added 3 commits December 16, 2024 17:35

Update RenameGroupView.java

d905fde

Update JabRef_en.properties

8bb211e

Update GroupTreeViewModel.java

27f5690

Rydzard marked this pull request as ready for review December 16, 2024 16:39

github-actions bot reviewed Dec 16, 2024

View reviewed changes

Merge branch 'main' into renameGroup

43a8654

This was referenced Feb 16, 2025

"Add subgroup" does not work on first time #12508

Closed

Update PULL_REQUEST_TEMPLATE.md #12509

Merged

koppor requested changes Feb 16, 2025

View reviewed changes

koppor closed this Feb 16, 2025

		private final EnumMap<GroupHierarchyType, String> hierarchyToolTip = new EnumMap<>(GroupHierarchyType.class);

		private final ControlsFxVisualizer validationVisualizer = new ControlsFxVisualizer();

Uh oh!

Conversation

Rydzard commented Nov 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mandatory checks

Uh oh!

Rydzard commented Nov 5, 2024

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Siedlerchr commented Nov 6, 2024

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Rydzard commented Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Siedlerchr commented Nov 19, 2024

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rydzard commented Nov 5, 2024 •

edited

Loading

Rydzard commented Nov 18, 2024 •

edited

Loading